feat(signin): Add conditional keys fetch, scopes in fxaOAuthLogin, "Authorization" state + in Strapi#20427
feat(signin): Add conditional keys fetch, scopes in fxaOAuthLogin, "Authorization" state + in Strapi#20427
Conversation
da50887 to
8ec1876
Compare
832952f to
6ec943b
Compare
d0e2e11 to
9231047
Compare
9cc46f9 to
d8f50c1
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates FxA Settings sign-in/sign-up integrations to support conditional key fetching (required vs opportunistic), propagates OAuth granted scopes back to Firefox via WebChannel, and generalizes the “signed into Firefox” state to enable an authorization-flow UI/CMS experience (including Strapi-driven copy).
Changes:
- Split key-fetch intent into
requiresKeysvswantsKeysIfPasswordEntered(withwantsKeys()becoming a combined signal) and update UI flow logic accordingly. - Add
scopesto thefxaOAuthLoginWebChannel payload (viagetGrantedScopes()). - Replace
isSignedIntoFirefoxDesktopwithisSignedIntoFirefox, addAuthorizePageCMS support, and add functional coverage for VPN authorization flow.
Reviewed changes
Copilot reviewed 38 out of 40 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/fxa-settings/src/pages/Signup/mocks.tsx | Updates Signup integration mocks to include requiresKeys. |
| packages/fxa-settings/src/pages/Signup/interfaces.ts | Extends Signup integration subset types with requiresKeys. |
| packages/fxa-settings/src/pages/Signup/container.tsx | Uses requiresKeys to gate passwordless signup redirect behavior. |
| packages/fxa-settings/src/pages/Signup/container.test.tsx | Updates Signup container tests for requiresKeys. |
| packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/interfaces.ts | Adds requiresKeys + getGrantedScopes to ConfirmSignupCode integration subsets. |
| packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/container.test.tsx | Updates ConfirmSignupCode tests to include requiresKeys in mocked integrations. |
| packages/fxa-settings/src/pages/Signin/utils.ts | Sends scopes in fxaOAuthLogin using integration.getGrantedScopes(). |
| packages/fxa-settings/src/pages/Signin/utils.test.ts | Adjusts Signin utils tests for updated key semantics (requiresKeys). |
| packages/fxa-settings/src/pages/Signin/mocks.tsx | Updates Signin mocks for new integration methods and Firefox-client detection helpers. |
| packages/fxa-settings/src/pages/Signin/interfaces.ts | Expands Signin integration subsets and renames prop to isSignedIntoFirefox. |
| packages/fxa-settings/src/pages/Signin/index.tsx | Refactors password-needed logic; adds authorization-flow detection and AuthorizePage CMS resolution; hides account switch in auth flow. |
| packages/fxa-settings/src/pages/Signin/index.test.tsx | Updates tests for isSignedIntoFirefox and new CMS/authorization-flow behavior. |
| packages/fxa-settings/src/pages/Signin/index.stories.tsx | Migrates to StoryObj format; adds authorization-flow stories; updates props/mocks. |
| packages/fxa-settings/src/pages/Signin/container.tsx | Prop rename plumbed from container to Signin component. |
| packages/fxa-settings/src/pages/Signin/SigninUnblock/mocks.tsx | Updates unblock mocks to include new integration fields. |
| packages/fxa-settings/src/pages/Signin/SigninTotpCode/mocks.tsx | Updates TOTP mocks for new integration fields and Firefox-client methods. |
| packages/fxa-settings/src/pages/Signin/SigninPushCode/mocks.tsx | Updates push-code mocks to include requiresKeys. |
| packages/fxa-settings/src/pages/PostVerify/SetPassword/container.test.tsx | Updates SetPassword tests to expect scopes in OAuth WebChannel payload. |
| packages/fxa-settings/src/pages/Index/mocks.tsx | Removes unused wantsKeys from Index mocks; formatting cleanup. |
| packages/fxa-settings/src/pages/Index/interfaces.ts | Removes wantsKeys from IndexIntegration type. |
| packages/fxa-settings/src/models/integrations/sync-basic-integration.ts | Renames Sync “keys required” signal from wantsKeys() to requiresKeys(). |
| packages/fxa-settings/src/models/integrations/relier-interfaces.ts | Adds AuthorizePage to CMS model. |
| packages/fxa-settings/src/models/integrations/oauth-web-integration.ts | Refactors scoped-keys detection into _scopeRequestsKeys(); exports scopeStrToArray. |
| packages/fxa-settings/src/models/integrations/oauth-native-integration.ts | Implements requiresKeys, wantsKeysIfPasswordEntered, and getGrantedScopes. |
| packages/fxa-settings/src/models/integrations/oauth-native-integration.test.ts | Adds tests for requiresKeys, opportunistic keys, and combined wantsKeys(). |
| packages/fxa-settings/src/models/integrations/integration.ts | Adds isFirefoxClient, splits key-fetch intent APIs, and introduces getGrantedScopes. |
| packages/fxa-settings/src/models/integrations/data/data.ts | Import formatting change (no functional change). |
| packages/fxa-settings/src/lib/oauth/hooks.tsx | Updates OAuth keys check hook to use requiresKeys() rather than wantsKeys(). |
| packages/fxa-settings/src/lib/integrations/integration-factory.test.ts | Updates integration-factory tests for requiresKeys, wantsKeys, and getGrantedScopes. |
| packages/fxa-settings/src/lib/channels/firefox.ts | Extends FxAOAuthLogin WebChannel type with optional scopes. |
| packages/fxa-settings/src/components/App/index.tsx | Renames and broadens Firefox sign-in state to isSignedIntoFirefox and plumbs it through routes. |
| packages/functional-tests/tests/misc/vpnIntegration.spec.ts | Adds functional test for VPN authorization flow (signed-in user, cached signin, scopes asserted). |
| packages/functional-tests/pages/layout.ts | Adds helper to assert WebChannel scopes presence in sent messages. |
| packages/functional-tests/lib/query-params.ts | Adds VPN mobile OAuth query params fixture. |
| libs/shared/cms/src/lib/queries/relying-party/types.ts | Adds AuthorizePage to relying-party CMS query types. |
| libs/shared/cms/src/lib/queries/relying-party/query.ts | Extends relying-party GraphQL query to fetch AuthorizePage. |
| libs/shared/cms/src/lib/queries/relying-party/factories.ts | Updates factory to generate AuthorizePage data. |
| libs/shared/cms/src/generated/graphql.ts | Regenerates GraphQL types to include AuthorizePage and schema changes. |
| libs/shared/cms/src/generated/gql.ts | Regenerates typed GraphQL document mappings for updated relying-party query. |
Files not reviewed (2)
- libs/shared/cms/src/generated/gql.ts: Language not supported
- libs/shared/cms/src/generated/graphql.ts: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // The user is in an authorization flow when they're signed into Firefox, | ||
| // it's a Firefox client (desktop or mobile), and a specific service is requested. | ||
| const isAuthorizationFlow = | ||
| isSignedIntoFirefox && | ||
| integration.isFirefoxClient() && | ||
| !!integration.getService(); |
There was a problem hiding this comment.
I'm going to leave this as-is. getService pulls from the query params, and we never have service=settings. integration.isFirefoxNonSync(); wouldn't work because a user could be signing into Sync later after they're already signed into VPN, Relay, or Smart Window, and technically they'd be authorizing Sync.
| isVpn, | ||
| }), | ||
| requiresKeys: () => false, | ||
| wantsKeys: () => true, | ||
| getCmsInfo: () => cmsInfo, |
| // granted scopes may differ from requested scopes. Update this to return | ||
| // the actual granted scopes from the server response. | ||
| getGrantedScopes(): string | undefined { | ||
| return this.data.scope; |
There was a problem hiding this comment.
This is fine and will be addressed in FXA-13495
…uthorization" state + in Strapi Because: * Non-Sync browser services (VPN, Relay, SmartWindow) should not force password entry just to fetch keys, but should fetch them opportunistically if a password is entered for another reason * The browser needs to know which scopes were granted after the authorization flow completes * The isSignedIntoFirefoxDesktop state was too narrow for the scope authorization flow which applies to all Firefox platforms, and we want this editable in Strapi This commit: * Splits wantsKeys into requiresKeys (Sync only, forces password) and wantsKeysIfPasswordEntered (non-Sync, opportunistic), with wantsKeys, to allow a "cached login" render without the "keys optional" capability, which is a capability intended for passwordless non-sync browser logins * Adds scopes field to fxaOAuthLogin WebChannel message at all call sites, because the browser needs to know which scopes were actually granted — with ADR 0049, FxA may deny requested scopes or grant additional ones, and the browser may not request scope at all * Renames isSignedIntoFirefoxDesktop to isSignedIntoFirefox and removes the Desktop-only check, adds a new page in Strapi for this state so the copy can be updated (e.g. "Authorize") * Adds tests/stories, updates Signin stories to new format closes FXA-12939
Because:
This commit:
closes FXA-12939 (see my comment on this ticket)
Strapi side: mozilla/fxa-strapi#219
The
wantsKeyschanges were needed because on main,wantsKeys()decided two things: "should we fetch keys" and "should we force the password." On browsers withkeys_optional(sync is decoupled), the password was already skipped for non-Sync browser services. But on browsers where Sync isn't decoupled yet (Fx 135-146 desktop, all current mobile),wantsKeys()forced the password for all browser services — including non-Sync ones where the user had already entered their password for Sync. The split lets us checkisSignedIntoFirefoxfor that case:requiresKeys()(Sync, always forces password),wantsKeysIfPasswordEntered()(non-Sync browser services, gets keys opportunistically), andwantsKeys()(are scoped keys being requested at all — used for keys: true to the auth server and unverified session routing).